feat(search): store SearchParameters in storage; registry becomes a TTL cache#244
Merged
Merged
Conversation
…TL cache (#235) Storage is now the source of truth for SearchParameters; the in-memory registry is a cache over it. - Seeding: on boot, each primary backend seeds the default tenant with the spec bundle (raw resource JSON, verbatim including status) plus the embedded fallbacks rendered as FHIR resources. Idempotent and multi-node safe: spec bundle ids make a concurrent second writer fail AlreadyExists (treated as seeded); a full set short-circuits to one count per boot; a partial set is completed without clobbering. GET SearchParameter now discovers the ~1380 parameters the server actually resolves, instead of an empty Bundle. - Refresh: refresh_stored_search_parameters() on the SQLite, Postgres, and Mongo backends rebuilds the registry Stored slice from storage (new SearchParameterRegistry::unregister_source primitive, which also re-promotes a spec param shadowed by a since-deleted override). Rows are fetched and parsed before the sync write lock is taken and the registry Arc identity is preserved (extractor / ES / chain builders hold clones); on a failed read the registry is left untouched, i.e. stale-serve, retried next tick. - HFS_SEARCH_PARAM_CACHE_TTL (default 3600, 0 disables) drives a periodic refresh task per serve path, so a SearchParameter POSTed to one cluster node becomes visible to the others within the TTL. In elasticsearch composite modes seed/refresh run on the primary, whose registry Arc the ES backend shares. - Mongo previously never reloaded stored SearchParameters after a restart (they only entered the registry via write hooks); its new refresh also runs at init_schema, closing that gap. Verified end to end on a file-backed server: first boot seeds 1377, GET SearchParameter?base=Patient returns spec entries, POST custom is 201, second boot takes the fast path (created=0) and reloads the stored custom param.
This was referenced Jul 10, 2026
…ed-search-params # Conflicts: # crates/hfs/src/main.rs
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Raises codecov/patch on #244 and fixes a latent inefficiency the added tests surfaced. - seeder: dedup the seed set by id before the count check. Several embedded fallbacks (Resource-id, Library-url, …) share an id with a spec bundle entry, so their create always fails AlreadyExists and never adds a row. Left in, they inflated resources.len() above the stored count the tenant can ever reach, so `present >= resources.len()` never held and every boot re-ran the full ~1380-create scan instead of the intended single-count fast path (observable as the second-boot "existing=1384" log). - loader: unit tests for load_spec_resources (happy + missing-dir error) and definition_to_fhir_resource (description/target branches, id derivation). - seeder tests: AlreadyExists slow-path over a present spec id, and the missing-spec-bundle (fallbacks-only) path. - mongodb: integration test for refresh_stored_search_parameters over the Mongo cursor path (parity with the SQLite refresh test). Verified: helios-fhir loader tests, the SQLite seeding suite, and the new Mongo refresh test (real testcontainer) all pass.
The flag added by #235 was only discoverable via `hfs --help` / source. Add it to the env-var tables alongside its siblings: the book's canonical reference, both READMEs, and the run-hfs-server skill.
smunini
previously approved these changes
Jul 17, 2026
Follow-up to #293. Replacing `-undefined dynamic_lookup` with `-Wl,-stack_size,0x800000` fixed the dyld crash but introduced a new build failure: ld64 rejects `-stack_size` on any link that is not a main executable — ld: -stack_size option can only be used when linking a main executable cargo applies rustflags to every link invocation, including proc-macro/cdylib crates (e.g. serde_derive, linked `-dynamiclib`), so the flag fails the build before any test runs. On Linux the equivalent `-zstack-size` is silently accepted for shared objects, which is why it was never caught there. macOS already defaults the main-thread stack to 8 MB — the same value the Linux side sets explicitly — so no override is needed. Drop the custom rustflags on macOS entirely. Claude-Session: https://claude.ai/code/session_01Vnu2A87mWaKnbeFuWZSwkd
smunini
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #235. Stacked on #242 (the
(base, code)shadowing fix) — per #239, that had to land first: with storage as the source of truth, every client-POSTed SearchParameter flows throughregister()with an arbitrary URL, and this PR's refresh relies on #242'sunregister_sourcesemantics (dropping a stored override re-promotes the spec param it shadowed).What changes
Seeding (storage becomes the source of truth). On boot each primary backend seeds the default tenant with the spec bundle's raw resource JSON plus the embedded fallbacks (new
definition_to_fhir_resource). Idempotent and multi-node safe, as the issue requires:Patient-name,Resource-id, …) make a concurrent second writer'screatefailAlreadyExists, treated as already-seeded — no duplication, no clobbering;countper boot;GET /SearchParameter?base=Patientnow returns the parameters the server actually resolves searches with, instead of the empty Bundle the issue demonstrated.Registry as a TTL cache. New
HFS_SEARCH_PARAM_CACHE_TTL(seconds, default 3600,0disables) spawns a periodic refresh per serve path.refresh_stored_search_parameters()on SQLite/Postgres/Mongo rebuilds the registry'sStoredslice from storage via the newSearchParameterRegistry::unregister_sourceprimitive. A SearchParameter POSTed to one cluster node becomes visible to the others within the TTL — the deliberate eventual consistency the issue specifies.Open questions from the issue, decided:
/metadatalag: accepted; a node's CapabilityStatement lags cluster-mates by up to the TTL, same as search resolution.params_by_typeoverride hazard: resolved by fix(search): resolve (base, code) shadowing by source precedence #242 underneath this PR.Design constraints honored (why the refresh looks like this)
Arcidentity is never swapped — the extractor, the ES shared registry, and both chain builders captured clones at construction and would silently serve a stale registry forever./metadata's single-guard build stays consistent.*-elasticsearchmodes, seed/refresh run on the primary, whose registry Arc the ES backend shares. The S3 path is unchanged (no settings/registry write hooks exist there today — pre-existing gap, unchanged by this PR).status: draftverbatim; the registry still loads spec definitions from the bundled file asEmbedded(draft→active promotion unchanged), so the stored copies serve discovery without becoming a second registration path. This is SearchParameter viewer & editor UI: Resource Filter rail, faceted browse, CRUD with registry-aware validation #238's open question 1 made explicit.init_schema.Multi-tenant note (flagging for review)
Seeding targets the default tenant because searches are tenant-scoped — seeding
__system__would leaveGET /SearchParameterempty, defeating the issue's first goal. Non-default tenants don't see the seeded resources via the API (the in-memory registry still resolves searches for every tenant, and the refresh reads cross-tenant exactly like today's boot load). If we later want shared-resource search (TenancyModel::Sharedexists but nothing consults it), that's a separate design.Verification
unregister_sourcedrops only its source and re-promotes a shadowed spec param.hfs: first boot seeds 1377 (failed=0),GET /SearchParameter?base=Patientreturns spec entries,POST /SearchParameter→ 201, second boot takes the fast path (created=0, existing=1384) and reloads the stored custom param.helios-fhir22/22, seeding suite 3/3, fullhelios-hfscheck clean withpostgres,mongodb,elasticsearch.